Update resources for plone.app.event #166

agitator · 2018-06-14T00:17:54Z

No description provided.

thet

LGTM

sunew · 2018-09-08T09:33:26Z

plone/app/upgrade/v51/configure.zcml

@@ -243,12 +243,19 @@ Add image scaling options to image handling control panel.
        destination="5113"
        profile="Products.CMFPlone:plone">

+        <gs:upgradeDepends
+            title="Run to512 upgrade profile."


@agitator title should be changed to "Run to513.."

sunew · 2018-09-09T09:44:52Z

@agitator FYI - I will fix the title and rebase on 5.1 (will do a rebase and a force push)

changelog

fixed

sunew · 2018-09-09T10:06:37Z

@jensens same procedure as "Remove jquery-highlightsearchterms" - except this one can go in 5.1.3 because it is independent of any additional changes to CMFPlone

sunew · 2018-09-09T10:57:29Z

jenkins 5.2 is currently failing for other reasons

sunew · 2018-09-13T10:07:27Z

@thet I'm on the failing test...

sunew · 2018-09-13T15:04:46Z

When running the tests of p.a.upgrade, on this PR together with
plone/plone.app.event#287 in 5.1 coredev, we get this error:

TypeError: "argument of type 'NoneType' is not iterable"
> /work/playground/plone/plone.coredev-5.1/src/Products.CMFPlone/Products/CMFPlone/resources/browser/combine.py(160)combine_bundles()
    159     container = get_override_directory(context)
--> 160     if PRODUCTION_RESOURCE_DIRECTORY not in container:
    161         container.makeDirectory(PRODUCTION_RESOURCE_DIRECTORY)

In the to50alpha1 step, when installing p.a.event
The TTW resource override directories are not available.

Full traceback:

Traceback (most recent call last):
  File "/work/buildout_cache/eggs/zope.testrunner-4.4.4-py2.7.egg/zope/testrunner/runner.py", line 400, in run_layer
    setup_layer(options, layer, setup_layers)
  File "/work/buildout_cache/eggs/zope.testrunner-4.4.4-py2.7.egg/zope/testrunner/runner.py", line 708, in setup_layer
    setup_layer(options, base, setup_layers)
  File "/work/buildout_cache/eggs/zope.testrunner-4.4.4-py2.7.egg/zope/testrunner/runner.py", line 713, in setup_layer
    layer.setUp()
  File "/work/buildout_cache/eggs/plone.app.testing-5.0.8-py2.7.egg/plone/app/testing/helpers.py", line 375, in setUp
    self.setUpPloneSite(portal)
  File "/work/playground/plone/plone.coredev-5.1/src/plone.app.upgrade/plone/app/upgrade/v50/testing.py", line 52, in setUpPloneSite
    portal.portal_migration.upgrade(swallow_errors=False)
  File "<string>", line 3, in upgrade
  File "/work/buildout_cache/eggs/AccessControl-3.0.14-py2.7-linux-x86_64.egg/AccessControl/requestmethod.py", line 70, in _curried
    return callable(*args, **kw)
  File "/work/playground/plone/plone.coredev-5.1/src/Products.CMFPlone/Products/CMFPlone/MigrationTool.py", line 294, in upgrade
    step['step'].doStep(setup)
  File "/work/playground/plone/plone.coredev-5.1/src/Products.GenericSetup/Products/GenericSetup/upgrade.py", line 166, in doStep
    self.handler(tool)
  File "/work/playground/plone/plone.coredev-5.1/src/plone.app.upgrade/plone/app/upgrade/v50/alphas.py", line 78, in to50alpha1
    qi.installProduct('plone.app.event')
  File "/work/playground/plone/plone.coredev-5.1/src/Products.CMFPlone/Products/CMFPlone/controlpanel/browser/quickinstaller.py", line 423, in installProduct
    return self.install_product(product_name)
  File "/work/playground/plone/plone.coredev-5.1/src/Products.CMFPlone/Products/CMFPlone/controlpanel/browser/quickinstaller.py", line 454, in install_product
    self.ps.runAllImportStepsFromProfile('profile-%s' % profile_id)
  File "/work/playground/plone/plone.coredev-5.1/src/Products.GenericSetup/Products/GenericSetup/tool.py", line 388, in runAllImportStepsFromProfile
    dependency_strategy=dependency_strategy)
   - __traceback_info__: profile-plone.app.event:default
  File "/work/playground/plone/plone.coredev-5.1/src/Products.GenericSetup/Products/GenericSetup/tool.py", line 1469, in _runImportStepsFromContext
    message = self._doRunImportStep(step, context)
  File "/work/playground/plone/plone.coredev-5.1/src/Products.GenericSetup/Products/GenericSetup/tool.py", line 1281, in _doRunImportStep
    return handler(context)
   - __traceback_info__: combine-bundles
  File "/work/playground/plone/plone.coredev-5.1/src/Products.CMFPlone/Products/CMFPlone/resources/exportimport/bundles.py", line 39, in combine
    combine_bundles(site)
  File "/work/playground/plone/plone.coredev-5.1/src/Products.CMFPlone/Products/CMFPlone/resources/browser/combine.py", line 160, in combine_bundles
    if PRODUCTION_RESOURCE_DIRECTORY not in container:
TypeError: argument of type 'NoneType' is not iterable

sunew · 2018-09-13T15:32:08Z

same problem as here: plone/Products.CMFPlone#1187

sunew · 2018-09-13T16:16:08Z

Problem described and solved in #1187 and plone/Products.CMFPlone#1412, was reintroduced in plone/Products.CMFPlone@932334391

We can either: do as in PR 1412 and make plone more forgiving (useful in upgrades and tests), or install the needed portal_resources, that means install plone.resource.
But I'm not sure what consequences this could have in a plone 5.0 (which the upgrade step applies to).

@thet @mauritsvanrees any opinions on this?

thet · 2018-09-13T21:18:59Z

@sunew awesome, I saw you fixed it!

At the same time I was fixing it too :D here is my git diff:

thet@thettop:/home/_thet/data/dev/plone/buildout.coredev-51/src/plone.app.upgrade$ git diff
diff --git a/plone/app/upgrade/v50/alphas.py b/plone/app/upgrade/v50/alphas.py
index 9ebc6d7..efc9915 100644
--- a/plone/app/upgrade/v50/alphas.py
+++ b/plone/app/upgrade/v50/alphas.py
@@ -66,7 +66,13 @@ def to50alpha1(context):
     # migrate properties to portal_registry
     migrate_registry_settings(portal)
 
-    # install plone.app.event
+    # update the default view of the Members folder
+    migrate_members_default_view(portal)
+
+    upgrade_keyring(context)
+    installOrUpgradePloneAppTheming(context)
+    installOrUpgradePloneAppCaching(context)
+
     try:
         from Products.CMFPlone.utils import get_installer
     except ImportError:
@@ -74,20 +80,15 @@ def to50alpha1(context):
         qi = getToolByName(portal, 'portal_quickinstaller')
     else:
         qi = get_installer(portal)
-    if not qi.isProductInstalled('plone.app.event'):
-        qi.installProduct('plone.app.event')
-
-    # update the default view of the Members folder
-    migrate_members_default_view(portal)
 
     # install the Barceloneta theme
     if portal.portal_skins.getDefaultSkin() == 'Sunburst Theme':
         if not qi.isProductInstalled('plonetheme.barceloneta'):
             qi.installProduct('plonetheme.barceloneta')
 
-    upgrade_keyring(context)
-    installOrUpgradePloneAppTheming(context)
-    installOrUpgradePloneAppCaching(context)
+    # install plone.app.event
+    if not qi.isProductInstalled('plone.app.event'):
+        qi.installProduct('plone.app.event')
 
 
 def installOrUpgradePloneAppTheming(context):
diff --git a/plone/app/upgrade/v51/final.py b/plone/app/upgrade/v51/final.py
index 0218a3b..825a91f 100644
--- a/plone/app/upgrade/v51/final.py
+++ b/plone/app/upgrade/v51/final.py
@@ -58,5 +58,10 @@ def remove_old_PAE_rescources(context):  # noqa
     """FORCE remove old p.a.event resources"""
     registry = getUtility(IRegistry)
     plone_legacy = registry.records['plone.bundles/plone-legacy.resources']
-    plone_legacy.value.remove('resource-plone-app-event-event-js')
-    plone_legacy.value.remove('resource-plone-app-event-event-css')
+
+    def rm_resource(resource):
+        if resource in plone_legacy.value:
+            plone_legacy.value.remove(resource)
+
+    rm_resource('resource-plone-app-event-event-js')
+    rm_resource('resource-plone-app-event-event-css')

Basically what differs from yours is that I install plone.app.event after plone.app.theming, which itself might install plone.resource. But installing plone.resouce explicitly is definitely nice.

If it runs green, go ahead an merge it.

sunew · 2018-09-14T07:09:07Z

@thet :D

we currently fail on plone 5.0 (when running all three PR's in the same jenkins run) because of this requirement in pa.event:

plone.app.z3cform>=2.0.1.dev0

Latest version of pa.event is not for plone 5.0 - and this PR is about upgrade steps for 5.1 - so I guess this failure is ok.

If we run this PR alone on 5.0 it should work - I'll do that. If it succeeds, I'll merge.

agitator requested a review from thet June 14, 2018 00:21

agitator force-pushed the p-a-event-to-513 branch from ba80d8c to 770e111 Compare June 14, 2018 08:32

thet approved these changes Jun 14, 2018

View reviewed changes

agitator mentioned this pull request Jun 14, 2018

Update resources plone/plone.app.event#287

Merged

sunew previously requested changes Sep 8, 2018

View reviewed changes

Update resources for plone.app.event

31dba25

changelog

sunew force-pushed the p-a-event-to-513 branch from 770e111 to 31dba25 Compare September 9, 2018 09:57

sunew requested a review from jensens September 9, 2018 10:05

sunew added the 01 type: bug label Sep 9, 2018

sunew requested a review from thet September 9, 2018 10:56

sunew requested a review from mauritsvanrees September 9, 2018 11:00

Dont fail if resource is already removed

8d02896

install plone.resource on upgrade, we need the portal_resources folder

c10780e

sunew merged commit c10780e into master Sep 14, 2018

sunew added a commit that referenced this pull request Sep 14, 2018

Merge pull request #166 from 'p-a-event-to-513'

425e049

sunew deleted the p-a-event-to-513 branch September 14, 2018 08:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update resources for plone.app.event #166

Update resources for plone.app.event #166

agitator commented Jun 14, 2018

thet left a comment

sunew Sep 8, 2018

sunew commented Sep 9, 2018

sunew commented Sep 9, 2018

sunew commented Sep 9, 2018

sunew commented Sep 13, 2018

sunew commented Sep 13, 2018 •

edited

Loading

sunew commented Sep 13, 2018

sunew commented Sep 13, 2018

thet commented Sep 13, 2018

sunew commented Sep 14, 2018

Update resources for plone.app.event #166

Update resources for plone.app.event #166

Conversation

agitator commented Jun 14, 2018

thet left a comment

Choose a reason for hiding this comment

sunew Sep 8, 2018

Choose a reason for hiding this comment

sunew commented Sep 9, 2018

sunew commented Sep 9, 2018

sunew commented Sep 9, 2018

sunew commented Sep 13, 2018

sunew commented Sep 13, 2018 • edited Loading

sunew commented Sep 13, 2018

sunew commented Sep 13, 2018

thet commented Sep 13, 2018

sunew commented Sep 14, 2018

sunew commented Sep 13, 2018 •

edited

Loading